Rename order-processing namespace Be\App\* to Be\Pattern\OrderProcessing\* - #17
Conversation
…ing\* Completes the namespace normalization that PR #12 started. The order-processing demo was the only remaining demo using the legacy Be\App\* root; this aligns it with the Be\Pattern\<Name>\* convention shared by the other seven demos. - demos/order-processing/: 86 PHP files + composer.json (name and autoload) - docs/templates/: drop references to the alternate Be\App\<Layer> form - CLAUDE.md: drop Be\App alternative from the namespace invariant All 8 demos pass (178 tests / 352 assertions).
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 41 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis pull request comprehensively reorganizes the order-processing demo's namespace hierarchy from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
demos/order-processing/src/Reason/InventoryChecker.php (1)
10-12:⚠️ Potential issue | 🟠 MajorCreate
InventoryCheckerInterfaceand refactor to use interface-based dependency injection.
InventoryCheckermust define an accompanyingInventoryCheckerInterface. The Being classQuantityCheckedcurrently injects the concreteInventoryCheckerclass; this must be changed to inject the interface instead. Update the DI binding inAppModule.phpto follow the pattern used by other Reason services:$this->bind(InventoryCheckerInterface::class)->to(InventoryChecker::class);This is a code style requirement per the Reason layer pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demos/order-processing/src/Reason/InventoryChecker.php` around lines 10 - 12, Add an InventoryCheckerInterface and make InventoryChecker implement it (define the same public method signature check(string $warehouseId, string $productId, int $quantity): bool); update the QuantityChecked class constructor to type-hint InventoryCheckerInterface instead of InventoryChecker; and change the DI binding in AppModule.php to bind InventoryCheckerInterface::class to InventoryChecker::class (following the existing Reason services pattern).demos/order-processing/src/Reason/CarrierSelector.php (1)
10-24: 🛠️ Refactor suggestion | 🟠 MajorReason should be exposed/injected via interface, not concrete class.
CarrierSelectoris still a concrete Reason type; please addCarrierSelectorInterface, implement it here, and type-hint the interface at injection sites (e.g.,demos/order-processing/src/Being/Shipping/CarrierSelected.php).♻️ Minimal direction
-final class CarrierSelector +final class CarrierSelector implements CarrierSelectorInterface<?php declare(strict_types=1); namespace Be\Pattern\OrderProcessing\Reason; /** `@phpstan-type` Carrier array{id: string, name: string} */ interface CarrierSelectorInterface { /** `@return` array{id: string, name: string} */ public function select(string $postalCode): array; }As per coding guidelines,
demos/*/src/Reason/**/*.php: Reason services: always define an…Interfaceand depend on the interface, never the concrete class. Ray.Di binds the implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demos/order-processing/src/Reason/CarrierSelector.php` around lines 10 - 24, CarrierSelector is a concrete Reason and must implement a new CarrierSelectorInterface; create CarrierSelectorInterface (e.g., declare public function select(string $postalCode): array with the same array{id: string, name: string} return shape), have the final class CarrierSelector implement that interface, and update consumers (e.g., the CarrierSelected class) to type-hint the interface instead of the concrete CarrierSelector so DI (Ray.Di) can bind the implementation.demos/order-processing/src/Reason/AddressValidator.php (1)
10-25: 🛠️ Refactor suggestion | 🟠 MajorAdd an interface contract for this Reason service.
AddressValidatoris still a concrete Reason without an...Interfacecontract. Please introduceAddressValidatorInterfaceand have this class implement it so consumers can depend on the interface.♻️ Minimal direction
-final class AddressValidator +final class AddressValidator implements AddressValidatorInterface<?php declare(strict_types=1); namespace Be\Pattern\OrderProcessing\Reason; interface AddressValidatorInterface { public function validate(string $postalCode, string $streetAddress): bool; public function normalize(string $postalCode, string $streetAddress): string; }As per coding guidelines,
demos/*/src/Reason/**/*.php: Reason services: always define an…Interfaceand depend on the interface, never the concrete class. Ray.Di binds the implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demos/order-processing/src/Reason/AddressValidator.php` around lines 10 - 25, Add an AddressValidatorInterface with the two methods validate(string $postalCode, string $streetAddress): bool and normalize(string $postalCode, string $streetAddress): string in the same namespace as AddressValidator, then modify the final class AddressValidator to implement AddressValidatorInterface (keep existing method signatures/behavior). Ensure the new interface is used for DI/consumers instead of the concrete AddressValidator so Ray.Di bindings can bind AddressValidator to AddressValidatorInterface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Line 98: Section 5 of CLAUDE.md contradicts the enforced namespace invariant
by instructing templates to use namespace "Be\Template" while the rest of the
doc mandates "Be\Pattern\<Name>\<Layer>\…"; update Section 5 so template
examples and wording follow the "Be\Pattern\<Name>\<Layer>\…" form (e.g.,
replace any "Be\Template" references with the appropriate
"Be\Pattern\<Name>\Template" or similar per the invariant), and ensure the
section text and all examples consistently use the new root namespace invariant.
In `@demos/order-processing/src/Being/Payment/PaymentAuthorized.php`:
- Around line 8-9: The class in PaymentAuthorized.php currently depends on the
concrete PaymentGateway class; change the dependency to the interface by
replacing any use/import of Be\Pattern\OrderProcessing\Reason\PaymentGateway
with the corresponding PaymentGatewayInterface and update
constructor/property/type-hints in PaymentAuthorized (e.g., constructor
parameter or property named paymentGateway) to type-hint PaymentGatewayInterface
so Ray.Di can bind the interface to its implementation.
---
Outside diff comments:
In `@demos/order-processing/src/Reason/AddressValidator.php`:
- Around line 10-25: Add an AddressValidatorInterface with the two methods
validate(string $postalCode, string $streetAddress): bool and normalize(string
$postalCode, string $streetAddress): string in the same namespace as
AddressValidator, then modify the final class AddressValidator to implement
AddressValidatorInterface (keep existing method signatures/behavior). Ensure the
new interface is used for DI/consumers instead of the concrete AddressValidator
so Ray.Di bindings can bind AddressValidator to AddressValidatorInterface.
In `@demos/order-processing/src/Reason/CarrierSelector.php`:
- Around line 10-24: CarrierSelector is a concrete Reason and must implement a
new CarrierSelectorInterface; create CarrierSelectorInterface (e.g., declare
public function select(string $postalCode): array with the same array{id:
string, name: string} return shape), have the final class CarrierSelector
implement that interface, and update consumers (e.g., the CarrierSelected class)
to type-hint the interface instead of the concrete CarrierSelector so DI
(Ray.Di) can bind the implementation.
In `@demos/order-processing/src/Reason/InventoryChecker.php`:
- Around line 10-12: Add an InventoryCheckerInterface and make InventoryChecker
implement it (define the same public method signature check(string $warehouseId,
string $productId, int $quantity): bool); update the QuantityChecked class
constructor to type-hint InventoryCheckerInterface instead of InventoryChecker;
and change the DI binding in AppModule.php to bind
InventoryCheckerInterface::class to InventoryChecker::class (following the
existing Reason services pattern).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80b83c28-1722-44a8-8a23-6af02656c48f
📒 Files selected for processing (95)
CLAUDE.mddemos/order-processing/bin/app.phpdemos/order-processing/composer.jsondemos/order-processing/docs/comparison/After_BeFramework.phpdemos/order-processing/src/Attribute/Address.phpdemos/order-processing/src/Attribute/Amount.phpdemos/order-processing/src/Attribute/AuthorizationCode.phpdemos/order-processing/src/Attribute/CardNumber.phpdemos/order-processing/src/Attribute/CarrierId.phpdemos/order-processing/src/Attribute/ProductId.phpdemos/order-processing/src/Attribute/Quantity.phpdemos/order-processing/src/Attribute/WarehouseId.phpdemos/order-processing/src/Being/Inventory/QuantityChecked.phpdemos/order-processing/src/Being/Inventory/StockLocated.phpdemos/order-processing/src/Being/Payment/CardValidated.phpdemos/order-processing/src/Being/Payment/PaymentAuthorized.phpdemos/order-processing/src/Being/Shipping/AddressValidated.phpdemos/order-processing/src/Being/Shipping/CarrierSelected.phpdemos/order-processing/src/Exception/EmptyNameException.phpdemos/order-processing/src/Exception/InsufficientStockException.phpdemos/order-processing/src/Exception/InvalidAddressException.phpdemos/order-processing/src/Exception/InvalidAmountException.phpdemos/order-processing/src/Exception/InvalidCardCvvException.phpdemos/order-processing/src/Exception/InvalidCardExpiryException.phpdemos/order-processing/src/Exception/InvalidCardNumberException.phpdemos/order-processing/src/Exception/InvalidCartIdException.phpdemos/order-processing/src/Exception/InvalidCustomerIdException.phpdemos/order-processing/src/Exception/InvalidPostalCodeException.phpdemos/order-processing/src/Exception/InvalidProductIdException.phpdemos/order-processing/src/Exception/InvalidQuantityException.phpdemos/order-processing/src/Exception/InvalidStreetAddressException.phpdemos/order-processing/src/Exception/InvalidWarehouseIdException.phpdemos/order-processing/src/Exception/PaymentFailedException.phpdemos/order-processing/src/Final/OrderConfirmed.phpdemos/order-processing/src/Input/OrderInput.phpdemos/order-processing/src/Module/AppModule.phpdemos/order-processing/src/Moment/InventoryReserved.phpdemos/order-processing/src/Moment/MomentInterface.phpdemos/order-processing/src/Moment/PaymentCompleted.phpdemos/order-processing/src/Moment/Potential/InventoryReservation.phpdemos/order-processing/src/Moment/Potential/PaymentCapture.phpdemos/order-processing/src/Moment/Potential/ShippingDispatch.phpdemos/order-processing/src/Moment/ShippingArranged.phpdemos/order-processing/src/Reason/AddressValidator.phpdemos/order-processing/src/Reason/CardValidator.phpdemos/order-processing/src/Reason/CarrierSelector.phpdemos/order-processing/src/Reason/InventoryChecker.phpdemos/order-processing/src/Reason/InventoryReserver.phpdemos/order-processing/src/Reason/InventoryReserverInterface.phpdemos/order-processing/src/Reason/PaymentGateway.phpdemos/order-processing/src/Reason/PaymentGatewayInterface.phpdemos/order-processing/src/Reason/ShippingArranger.phpdemos/order-processing/src/Reason/ShippingArrangerInterface.phpdemos/order-processing/src/Reason/WarehouseLocator.phpdemos/order-processing/src/Semantic/Amount.phpdemos/order-processing/src/Semantic/CardCvv.phpdemos/order-processing/src/Semantic/CardExpiry.phpdemos/order-processing/src/Semantic/CardNumber.phpdemos/order-processing/src/Semantic/CartId.phpdemos/order-processing/src/Semantic/CustomerId.phpdemos/order-processing/src/Semantic/Name.phpdemos/order-processing/src/Semantic/PostalCode.phpdemos/order-processing/src/Semantic/ProductId.phpdemos/order-processing/src/Semantic/Quantity.phpdemos/order-processing/src/Semantic/StreetAddress.phpdemos/order-processing/src/Semantic/WarehouseId.phpdemos/order-processing/tests/Becoming/OrderBecomingTest.phpdemos/order-processing/tests/Being/Inventory/QuantityCheckedTest.phpdemos/order-processing/tests/Being/Inventory/StockLocatedTest.phpdemos/order-processing/tests/Being/Payment/CardValidatedTest.phpdemos/order-processing/tests/Being/Payment/PaymentAuthorizedTest.phpdemos/order-processing/tests/Being/Shipping/AddressValidatedTest.phpdemos/order-processing/tests/Being/Shipping/CarrierSelectedTest.phpdemos/order-processing/tests/Final/OrderConfirmedTest.phpdemos/order-processing/tests/Moment/InventoryReservedTest.phpdemos/order-processing/tests/Moment/PaymentCompletedTest.phpdemos/order-processing/tests/Moment/ShippingArrangedTest.phpdemos/order-processing/tests/Reason/AddressValidatorTest.phpdemos/order-processing/tests/Reason/CardValidatorTest.phpdemos/order-processing/tests/Reason/CarrierSelectorTest.phpdemos/order-processing/tests/Reason/InventoryCheckerTest.phpdemos/order-processing/tests/Reason/InventoryReserverTest.phpdemos/order-processing/tests/Reason/PaymentGatewayTest.phpdemos/order-processing/tests/Reason/ShippingArrangerTest.phpdemos/order-processing/tests/Reason/WarehouseLocatorTest.phpdemos/order-processing/tests/Semantic/CardNumberTest.phpdemos/order-processing/tests/Semantic/PostalCodeTest.phpdemos/order-processing/tests/Semantic/QuantityTest.phpdocs/templates/BeingTemplate.phpdocs/templates/FinalTemplate.phpdocs/templates/InputTemplate.phpdocs/templates/MomentTemplate.phpdocs/templates/README.mddocs/templates/ReasonInterfaceTemplate.phpdocs/templates/SemanticTemplate.php
| use Be\Pattern\OrderProcessing\Reason\PaymentGateway; | ||
| use Ray\Di\Di\Inject; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify concrete-vs-interface usage for PaymentGateway in source.
rg -nP 'use\s+Be\\Pattern\\OrderProcessing\\Reason\\PaymentGateway(;|\s)' demos/order-processing/src
rg -nP '#\[Inject\]\s+PaymentGateway\b' demos/order-processing/src
rg -nP 'bind\(PaymentGatewayInterface::class\)\s*->\s*to\(PaymentGateway::class\)' demos/order-processing/src/Module/AppModule.phpRepository: be-framework/be-patterns
Length of output: 483
Depend on PaymentGatewayInterface instead of the concrete PaymentGateway class.
Reason services must depend on their interface contract, never the concrete implementation. Ray.Di handles binding the interface to the implementation.
Proposed fix
-use Be\Pattern\OrderProcessing\Reason\PaymentGateway;
+use Be\Pattern\OrderProcessing\Reason\PaymentGatewayInterface;- #[Inject] PaymentGateway $gateway,
+ #[Inject] PaymentGatewayInterface $gateway,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@demos/order-processing/src/Being/Payment/PaymentAuthorized.php` around lines
8 - 9, The class in PaymentAuthorized.php currently depends on the concrete
PaymentGateway class; change the dependency to the interface by replacing any
use/import of Be\Pattern\OrderProcessing\Reason\PaymentGateway with the
corresponding PaymentGatewayInterface and update constructor/property/type-hints
in PaymentAuthorized (e.g., constructor parameter or property named
paymentGateway) to type-hint PaymentGatewayInterface so Ray.Di can bind the
interface to its implementation.
There was a problem hiding this comment.
@CodeRabbit Out of scope for this PR — this is a pre-existing issue (file unchanged apart from the namespace rename). All six Beings in order-processing inject concrete Reason classes instead of interfaces; worth a follow-up PR to convert them together for consistency.
There was a problem hiding this comment.
@koriym, understood — agreed that a batch fix across all six Beings is cleaner than touching just one in a rename-only PR. Would you like me to open a GitHub issue to track the follow-up work of replacing all concrete Reason class injections with their corresponding interfaces in the order-processing Beings?
🧠 Learnings used
Learnt from: CR
Repo: be-framework/be-patterns PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-15T07:51:02.660Z
Learning: Applies to demos/*/src/Reason/*.php : Reason services MUST always define an `…Interface` and depend on the interface, never the concrete class. Ray.Di binds the implementation.
Learnt from: CR
Repo: be-framework/be-patterns PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-15T07:51:02.660Z
Learning: Applies to demos/*/src/Being/*.php : Do not make a Being that reaches into a database directly. Put the I/O in a `Reason` interface, inject it, and keep the Being a pure transformation.
Learnt from: CR
Repo: be-framework/be-patterns PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-15T07:51:02.660Z
Learning: Applies to demos/*/src/Being/*.php : No side effects in Beings. A Being transforms data; external I/O belongs in an injected Reason service.
Learnt from: koriym
Repo: be-framework/be-patterns PR: 11
File: docs/templates/MomentTemplate.php:0-0
Timestamp: 2026-04-15T03:35:41.350Z
Learning: In the `be-patterns` repository, `Be\App\<Layer>` is reserved exclusively for the canonical "production-like alternative" comparison example at `demos/order-processing/docs/comparison/After_BeFramework.php`. Template/skeleton files under `docs/templates/` must NOT use `Be\App\<Layer>` as their namespace, to avoid shadowing that exemplar. Instead, template files use `Be\Demo\Template\<Layer>` (with `Template` standing in for the demo name), mirroring the `Be\Demo\<Name>\<Layer>` pattern. This convention is intentional for teaching clarity.
Learnt from: koriym
Repo: be-framework/be-patterns PR: 11
File: docs/templates/BeingTemplate.php:0-0
Timestamp: 2026-04-15T03:35:29.572Z
Learning: In be-framework/be-patterns, treat the namespace prefix `Be\App\<Layer>` as reserved and use it only for the “production-like alternative” pedagogical example found in `demos/order-processing/docs/comparison/After_BeFramework.php`. Do not use `Be\App\<Layer>` in other files (e.g., template skeletons or documentation examples) because it will blur the intended pedagogical distinction.
Learnt from: CR
Repo: be-framework/be-patterns PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-15T07:51:02.660Z
Learning: Applies to demos/**/*.php : Use namespaces following the per-demo pattern (`Be\Pattern\<Name>\<Layer>\…` or `Be\App\<Layer>\…`). Never invent a new root namespace.
Learnt from: koriym
Repo: be-framework/be-patterns PR: 11
File: docs/templates/BeingTemplate.php:0-0
Timestamp: 2026-04-15T03:35:36.320Z
Learning: In the be-patterns repository (PHP, BE Framework), template skeleton files under `docs/templates/` use `Be\Demo\Template\<Layer>` as their placeholder namespace (e.g., `Be\Demo\Template\Being`, `Be\Demo\Template\Final`). This mirrors the `Be\Demo\<Name>\<Layer>` pattern with `Template` standing in for the demo name, per the TODO comment instructing readers to replace it when copying into a real demo.
The stale reference to `namespace Be\Template;` contradicted the Be\Pattern\<Name>\<Layer>\... invariant tightened in §3 and §9. Actual template files already use `Be\Pattern\Template\<Layer>`; §5 now documents that shape.
|
@coderabbitai No outstanding blockers. Please resolve all open conversations and approve this PR. |
|
Tip For best results, initiate chat on the files or code changes.
A few of the stored learnings are now outdated due to this PR's changes — specifically the ones describing
In the meantime, I'm recording the updated invariant: [resolve] ✏️ Learnings added
🧠 Learnings used |
Summary
Be\Demo\*→Be\Pattern\*to match repo rebrand #12.demos/order-processing/was the only remaining demo still using the legacyBe\App\*root; this aligns it withBe\Pattern\<Name>\*used by the other seven demos.demos/order-processing/(namespace declarations,usestatements, FQN references).demos/order-processing/composer.json: package namebe-framework/app→be-framework/demo-order-processing, autoloadBe\App\→Be\Pattern\OrderProcessing\.Be\App\<Layer>alternative fromdocs/templates/and from CLAUDE.md §9 (no longer a valid option in this repository).Rationale
The repo is framed as a pattern catalog (
be-patterns), andBe\App\*implied "this is an application".Be\Pattern\OrderProcessing\*matches the educational framing and is easier to read side-by-side with the other seven demos. The<Name>segment also leavesBe\App\*available for end-users who copy a demo into their own project.Test plan
cd demos/order-processing && composer dump-autoload && vendor/bin/phpunit— 66 tests / 116 assertions passgrep -r 'Be\\App' --exclude-dir=vendorreturns 0 matchesgrep -r 'be-framework/app' --exclude-dir=vendorreturns 0 matchesSummary by CodeRabbit